-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: add support for UpgradePolicy attribute in cluster creation #8534
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
b00f0d6
to
6927aa3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good, have a couple small comments around validation. Thanks for the contribution
ae878d6
to
e04f40b
Compare
@NicholasBlaskey Just made an update to address issues that you pointed out - e04f40b thanks ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, just seeing a build failure from one of the lines.
Could we get that resolved? I think we should be good after that
6cf02eb
to
a6c8769
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry one more comment came up when testing this out
# UpgradePolicy allows you to specify the support type for your cluster | ||
# Valid values are "STANDARD" and "EXTENDED (default)" | ||
# - https://docs.aws.amazon.com/eks/latest/APIReference/API_UpgradePolicyRequest.html | ||
upgradePolicy: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we move upgradePolicy
outside of metadata and put it as a top level field?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NicholasBlaskey thanks for your review, I simplified the sample config - 3ba2905 please review again, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we move upgradePolicy
out of metadata and make it on the same level like managedNodegroups
For example so the user would have to specify
metadata:
name: upgrade-policy-cluster
region: us-west-2
upgradePolicy:
supportType: "EXTENDED"
managedNodeGroups:
- name: mng-1
desiredCapacity: 1
We will need to update both the example and the code to account for this change. This change would help match the structure of the create cluster EKS API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh... Yes, I misunderstood what you mean.
Will do, thanks!
3a5295a
to
bddce1a
Compare
bddce1a
to
dd6a97f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks for the contribution
Adds support for UpgradePolicy attribute during cluster creation, allowing users to specify support type (STANDARD or EXTENDED) for EKS clusters.
Resolves #7932